check-msrv: Autodetect expected MSRV #429
Conversation
This commit updates the `check-msrv` script to autodetect the expected minimum supported Rust version (MSRV) for each crate. This is done by querying the `rust_version` field of the package metadata using `jq`. This ensures that the script always tests the correct MSRV for each crate, even if the MSRV is updated in the future.
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script
participant Cargo
User->>Script: Execute check-msrv.sh
Script->>Script: Call check(package_name)
Script->>Cargo: Update workspace member
Cargo-->>Script: Confirm update
Script->>Cargo: Retrieve MSRV
Cargo-->>Script: Return MSRV
Script->>Cargo: Run tests with MSRV
Cargo-->>Script: Test results
Script-->>User: Output results
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
check-msrv.sh (1)
Line range hint
1-18: Consider enhancing script robustness and maintainability.While the basic error handling is in place, there are several improvements that could make the script more robust:
- Add script documentation explaining its purpose and usage
- Add version checking for required tools (cargo, jq)
- Consider using command-line argument parsing for better flexibility
- Add logging levels for better debugging
Here's a suggested improvement for the script header:
+#!/usr/bin/env bash +# check-msrv.sh: Test packages against their Minimum Supported Rust Version (MSRV) +# Usage: ./check-msrv.sh [--verbose] + set -Ceu +# Check required tools +for cmd in cargo jq git; do + if ! command -v "$cmd" >/dev/null 2>&1; then + echo "Error: Required command '$cmd' not found" >&2 + exit 1 + fi +done + if [ "$*" = "" ]; then quiet='--quiet'; else quiet=''; fi
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
check-msrv.sh (2)
30-31: Consider making the jq query more robust.While the current query works with
set -e, it could be made more robust by providing a fallback value when therust_versionfield is missing:- msrv=$(cargo metadata --format-version=1 | - jq -r ".packages[] | select(.name == \"$package\") | .rust_version") + msrv=$(cargo metadata --format-version=1 | + jq -r ".packages[] | select(.name == \"$package\") | .rust_version // \"1.70.0\"")This ensures a sensible default MSRV if the field is missing, while maintaining the script's behavior due to
set -e.
38-48: Consider extracting package list for better maintainability.While the current implementation works well, consider extracting the package list to make it more maintainable:
+# List of packages to test +packages="yash-arith yash-builtin yash-cli yash-env yash-env-test-helper \ + yash-executor yash-fnmatch yash-prompt yash-quote yash-semantics" + +# Test regular packages +for package in $packages; do + check "$package" +done + +# Test yash-syntax with and without features +check yash-syntax '' '--features annotate-snippets'This makes it easier to:
- Add or remove packages in the future
- See all packages at a glance
- Maintain special cases separately
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
check-msrv.sh(1 hunks)
🔇 Additional comments (3)
check-msrv.sh (3)
19-21: LGTM! Clear and concise function documentation.
The documentation clearly describes the function parameters and their purpose.
22-26: LGTM! Robust parameter handling.
The parameter handling is POSIX-compliant and handles the case of no additional options gracefully.
33-35: LGTM! Clean and POSIX-compliant test execution.
The loop correctly handles multiple test options while maintaining the quiet flag functionality.
This commit updates the
check-msrvscript to autodetect the expectedminimum supported Rust version (MSRV) for each crate. This is done by
querying the
rust_versionfield of the package metadata usingjq.This ensures that the script always tests the correct MSRV for each
crate, even if the MSRV is updated in the future.
Summary by CodeRabbit
New Features
checkfunction to streamline workspace member updates and dynamically retrieve the minimum supported Rust version (MSRV) for packages.Improvements
checkfunction.Bug Fixes